-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(deps): Move build dependencies to non-dev dependencies #1107
Conversation
@queengooborg This timeout is weird. Were you able to run the tests locally?
|
I was not, and it's actually failing on the |
@queengooborg Did you check (and how) if those two dependencies are all that need to be moved? |
I checked by running the following: cd mdn-interactive-examples
npx install-local ../mdn-bob
npm run build |
This pull request has merge conflicts that must be resolved before it can be merged. |
This comment was marked as outdated.
This comment was marked as outdated.
It's But I'm still getting an error 😕:
|
Oops, wrote that reply from my phone and forgot the command for a second... 😆
That's odd, I wonder why that error didn't pop up on my end... |
bee9488
to
efab79c
Compare
Ah, I realize now what I was doing wrong; I was running Anyways, I've also updated the test to also perform a clean install before building, and then install the dev dependencies before running the tests with Jest. It looks like that was the last dependency that shouldn't have been a dev dependency! |
@queengooborg Thank you. I have got another error now though: Here's what I did: cd bob
npm run build
cd ../interactive-examples
npx install-local
npm run build |
😠 It looks like this has something to do with folder resolution with Webpack -- I'll submit another PR to fix that up to keep the scope of this PR down! |
This is now all that's left to get BoB fixed in production! |
🍿 |
This PR moves dependencies required for building from the devDependencies to regular dependencies.